Pull request overview

This PR migrates DateRangeCalendar’s drag-to-edit interaction from native HTML5 drag-and-drop (plus a separate touch path) to a unified Pointer Events implementation, improving mobile UX (notably eliminating iOS Safari’s long-press drag delay).

Changes:

• Rewrote useDragRange to drive range endpoint dragging via pointerdown/pointerover with document-level pointerup commit handling and scroll suppression during the gesture.
• Updated DateRangePickerDay draggable styling/behavior to align with the new pointer-driven gesture and avoid native side-effects (selection/callouts).
• Updated unit tests and test utilities to replay drags using pointer events instead of drag/touch events.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

PR Review — mui/mui-x #22279

Title: [DateRangeCalendar] Use Pointer Events for drag editing
URL: #22279
Scope: 4 files, +178/−503 — full rewrite of useDragRange.ts, drops the parallel HTML5-drag + touch code paths, unifies under Pointer Events.

Reviewed via four parallel agents (code-reviewer, silent-failure-hunter, pr-test-analyzer, comment-analyzer), cross-referenced against the range-calendar branch on base-ui-plus which implements the same feature with a root-level pointer-capture + elementFromPoint architecture.

Files referenced below are paths within the PR repo (mui-x):

• packages/x-date-pickers-pro/src/DateRangeCalendar/useDragRange.ts
• packages/x-date-pickers-pro/src/DateRangeCalendar/DateRangeCalendar.test.tsx
• packages/x-date-pickers-pro/src/DateRangePickerDay/DateRangePickerDay.tsx
• test/utils/pickers/calendar.ts

Critical (must fix before merge)

The architectural root cause beneath all three: the hook treats pointerup as a reliable terminator, which it is not on touch/pen hardware. Cleanup is keyed off pointerId equality with no watchdog, no lostpointercapture, no recovery path.

C1. Unmount mid-drag leaves state stuck

useDragRange.ts:256-261. The useEffect cleanup only removes document listeners; it does not reset isDraggingRef, pointerIdRef, pendingDropRef, sourceDateRef. If the component re-mounts (month nav, parent re-render of a different cell tree) pointerIdRef.current != null is still true → the handlePointerDown guard at L126 rejects every subsequent gesture. Also keeps pendingDropRef.current.target (an unmounted DOM node) alive.

Fix: null all refs in the unmount cleanup.

React.useEffect(
  () => () => {
    cleanupListenersRef.current?.();
    cleanupListenersRef.current = null;
    pointerIdRef.current = null;
    isDraggingRef.current = false;
    sourceDateRef.current = null;
    sourcePositionRef.current = null;
    didMoveRef.current = false;
    pendingDropRef.current = null;
  },
  [],
);

C2. No watchdog for lost pointerup — permanent jam

useDragRange.ts:208-216, :126. If the OS swallows both pointerup and pointercancel for the active pointerId (documented iOS / WebView quirk under system gesture, page hide, capture steal), document listeners stay attached forever and the L126 guard refuses every future gesture. The comment at L124-125 names "recovery from a lost pointerup" as a goal of the pointerIdRef check, but the check only refuses recovery rather than enabling it.

Fix (preferred): on the L126 guard hit, proactively call cleanup() and start the new gesture — a fresh primary pointerdown definitionally ends the previous one. Reference branch sidesteps this entirely via setPointerCapture on the root + lostpointercapture.

C3. pointercancel after a real move silently discards the drop

useDragRange.ts:193-198. When the user has dragged to a new cell, pointercancel (iOS system gesture, context-menu long-press, scroll-snap interrupt) calls cleanup() which unwinds UI but never fires onDrop. From the user's perspective: range snaps back, no explanation, no recovery.

Fix: treat pointercancel after didMoveRef === true identically to pointerup and commit the drop, OR expose an onDragCancel(sourceDate, lastPreviewDate) callback. Spec intent of pointercancel is "UA interrupted the gesture, not the user".

Important

I1. Drag listeners bind to top-level document instead of ownerDocument

useDragRange.ts:181-184, :208-215. Breaks iframe-hosted pickers. The reference branch (RangeCalendarStore.handlePointerMove) routes everything through ownerDocument(this.rootRef.current).

Fix: const ownerDoc = event.currentTarget.ownerDocument ?? document; at the top of handlePointerDown, use for all four listeners and the click suppressor.

I2. No Escape-key cancellation while a drag is in progress

useDragRange.ts — useDragRangeEvents, no keydown handler at all. Native HTML5 DnD provided this for free; pointer-events implementations must wire it explicitly. Accessibility regression. Reference branch wires this in RangeCalendarStore.handleKeyDown.

Fix: add a document-level keydown listener in handlePointerDown that calls cleanup() on Escape.

I3. iOS text-selection magnifier not suppressed

useDragRange.ts:115-217. WebkitTouchCallout: none (DateRangePickerDay.tsx:228) handles the callout, but the iOS magnifier is a separate feature; the canonical fix is event.preventDefault() inside pointerdown. Reference impl does this.

Fix: add event.preventDefault() after the stopPropagation() at L145.

I4. releasePointerCapture unguarded against DOMException

useDragRange.ts:138-143. Safari 15 / some Android WebViews throw InvalidPointerId between the hasPointerCapture check and the releasePointerCapture call (non-atomic). The throw escapes handlePointerDown and the gesture silently fails to start. Reference branch wraps both calls in try/catch.

Fix: wrap in try/catch — it's a benign "already released" race.

I5. touchmove listener installed even on tap-without-move

useDragRange.ts:200-210. Non-passive touchmove on document is registered at pointerdown, before any movement. For every endpoint tap (very common — that's how you advance the range), the page loses compositor-thread scrolling until the user lifts.

Fix: defer the touchmove listener registration until didMoveRef flips to true inside handlePointerOver. The internal isDraggingRef check then becomes redundant.

I6. onDatePositionChange silently skipped if data-position missing

useDragRange.ts:244-248, :153-154. If a custom day-slot strips data-position, the cast (position as RangePosition | undefined) ?? null resolves to null, the dispatch is silently skipped, but setIsDragging(true) still fires — preview computes against the wrong endpoint with no diagnostic.

Fix: when sourcePositionRef.current is null at activation time, either abort drag entry or console.warn in dev mode.

I7. resolveDateFromTarget throws RangeError on non-numeric data-timestamp

useDragRange.ts:49-66. new Date(NaN).toISOString() throws; Number(non-numeric) is NaN. Throw mid-pointerover wedges the gesture and never reaches cleanup().

Fix: guard Number.isFinite(timestamp) before passing to adapter.date; return null otherwise.

I8. Two-listener race in synthetic-click suppressor

useDragRange.ts:176-184. Rapid back-to-back drags can leave two suppressClick closures racing on document; a stray unrelated click in the macrotask window can be silently consumed by the wrong gesture's suppressor.

Fix: track outstanding suppressors in a ref and replace, or rely on { once: true } + the setTimeout as belt-and-suspenders only.

Test gaps

The PR's claim that "mouse, touch and pen flow through one code path so one test suite suffices" is true for the main handlePointerDown body, false for the touch-specific defenses (implicit capture release, document-level touchmove, jsdom hasPointerCapture guard). Deleting the touch-specific tests without adding behavioural assertions for these branches is the central testing concern.

Other test issues

• T7. The new "child elements" test is a near no-op — DateRangeCalendar.test.tsx:247-273. React delegation supplies event.currentTarget = button regardless of which child the user touched, so getClosestElementWithDataAttribute short-circuits on the button itself and never walks. Either rewrite the production code to read event.target, or rewrite the test to call the helper directly, or delete it.
• T8. executeDateDrag fires pointerUp on document — calendar.ts:24. Technically valid (production listens on document) but a regression that moves the listener back to the cell would silently keep passing in jsdom but break in real browsers. Consider firing pointerup on the last otherDate and letting it bubble.
• T9. Missing clientX/clientY on synthetic pointer events — ticking time-bomb if a drag-threshold is ever added.
• T10. pointerId: 1 magic number duplicated in calendar.ts and the test file (instead of importing the helper constant).
• T11. Repo-wide grep needed for stale buildPickerDragInteractions callers — public test-utility rename.

Comment issues

Inaccurate

• useDragRange.ts:116-117 — The > 0 rationale is fabricated. jsdom's MouseEventInit.button defaults to 0, not undefined. The real reason to prefer > 0 is that real browsers send button: -1 on events where no button changed state (e.g. pointerover while a primary press is held); !== 0 would falsely flag those. Rewrite around that.
• DateRangeCalendar.test.tsx:248-250 — "The handler must walk up to the button to read its data attributes — exercise that path explicitly." The handler reads event.currentTarget (the bound button), so no walking happens. Comment is rotted relative to the code it documents.
• useDragRange.ts:122-125 — "The pointerIdRef check also covers pen+touch (each pointer type has its own primary)" — slightly misleading: pen+touch is covered only by pointerIdRef, not by the isPrimary half. Tighten.

Coupling / rot risk

• useDragRange.ts:219-221 — Drop the "testing-library fireEvent" half. The production rationale (React synthesizes enter/leave from over/out) is durable and sufficient.
• useDragRange.ts:156-158 — References handlePointerOver and rangePosition by name; neither will survive a refactor cleanly. Generic phrasing is fine.
• calendar.ts:5-11 — Docstring above executeDateDragWithoutDrop says "...then pointerup", but that variant explicitly does not fire pointerup. Trim.

Good (keep)

The bulk of the comments do real work: the implicit-capture release rationale (L135-137), the cleanup re-render skip (L106), the swallow-click strategy (L172-175), the touch-action-isn't-enough explanation (L200-202), and the camelCase/kebab-case JSDoc on getClosestElementWithDataAttribute (L32-36) are all exactly the kind of comments that earn their keep in this file.

Strengths

• Sound state machine. Each ref has a single documented purpose; cleanup is centralised.
• Defer-drag-UI-until-first-move cleanly separates tap-on-endpoint from drag-from-endpoint — the comment at L156-158 documents this invariant.
• pendingDropRef.current?.target === event.currentTarget early-return prevents redundant state updates on repeated pointerover.
• hasPointerCapture feature-detect correctly handles jsdom.
• Document-level touchmove.preventDefault closes the iOS scroll-while-drag gap that touch-action: none alone leaves once the finger crosses cell boundaries.
• Removed dead code (emptyDragImgRef, MockedDataTransfer, resolveButtonElement, resolveElementFromTouch, rangeCalendarDayTouches coord table) — net simplification.
• No as any casts introduced. The RangePosition cast is properly guarded with ?? null.
• Flip / reduce / expand happy paths and shouldDisableDate mid-drag reactivity are well covered.

Recommended action plan

1. Block on: C1, C2, C3 (critical state-machine bugs).
2. Address before merge: I1 (ownerDocument), I2 (Escape), I4 (releasePointerCapture try/catch), I7 (Number.isFinite guard).
3. Add tests: T1, T2, T3 minimum; T5/T6 strongly recommended. Decide T7 (rewrite vs. delete).
4. Comment fixes: the three inaccurate items.
5. Architectural note (not blocking): the reference range-calendar branch (base-ui-plus) uses root-level setPointerCapture + elementFromPoint, which sidesteps C2/C3 by design and gets lostpointercapture as a free recovery signal. If you're going to ship more pointer-driven gestures in mui-x pickers, that pattern is worth lifting wholesale rather than continuing per-hook cleanup orchestration.
6. Manual cross-browser smoke required: iOS Safari 15/16/17, Android Chrome. jsdom does not simulate pointer-capture semantics — the releasePointerCapture + pointerover-on-siblings load-bearing assumption is unverifiable in CI.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

    // Ignore secondary mouse buttons. `> 0` (not `!== 0`) is intentional:
    // real browsers report `button: -1` on events where no button changed
    // state, and we want to treat those as primary.
    if (event.button > 0) {

    const { position } = event.currentTarget.dataset;
    sourcePositionRef.current = (position as RangePosition | undefined) ?? null;

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

    if (eventType === 'pointerup' && wasMoved) {
      // The click that follows pointerup would re-enter the day's selection
      // logic and undo the drop; swallow it. (Not needed on pointercancel —
      // no click follows a canceled gesture.)
      installClickSuppressor(ownerDoc);
    }

  // On unmount, clear gesture state so a remount can start fresh and any
  // detached DOM nodes still referenced by gesture refs can be GC'd.
  // `clearGestureState` is `useEventCallback`-stable, so the effect runs once.
  React.useEffect(() => () => clearGestureState(), [clearGestureState]);

    const onKeyDown = (keyEvent: KeyboardEvent) => {
      if (keyEvent.key !== 'Escape') {
        return;
      }
      // Only consume Escape when there's a visible drag in flight. A press
      // without movement is indistinguishable from a tap; let Escape
      // propagate so a host modal/popover can still close on the same key.
      if (didMoveRef.current) {
        keyEvent.preventDefault();
      }
      cleanup();
    };

    // Ignore secondary mouse buttons (middle = 1, right = 2). `> 0` rather
    // than `!== 0` keeps the gesture permissive when `event.button` is left
    // unset by a synthetic event (some test environments).
    if (event.button > 0) {
      return;
    }

👍 Looks good to me overall. The Pointer Events state machine is clear and the new tests cover the important edge cases. One small test coverage point:

(1) The disableDragEditing test (and the draggable attribute parts of the readOnly / disabled tests) no longer verifies behavior — draggable is removed before reaching the DOM.